#!/bin/bash
echo "=== src/routes/players.rs: map_err and Err patterns ==="
rg -n 'map_err|Err\(' src/routes/players.rs

echo ""
echo "=== tests: error pattern-matching on read/delete results ==="
rg -n 'is_ok\(\)|is_none\(\)|unwrap\(\)|is_err\(\)|PlayerServiceError|Database(' tests/

#!/bin/bash
echo "=== tests: error and result assertion patterns ==="
rg -n 'is_ok\(\)|is_none\(\)|unwrap\(\)|is_err\(\)|PlayerServiceError|Database\(' tests/

Learnt from: nanotaboada
URL: https://github.com/nanotaboada/rust-samples-rocket-restful/pull/82

Timestamp: 2026-04-08T14:40:53.512Z
Learning: In `src/services/player_service.rs` (as of PR `#82`), the functions `get_all`, `get_by_id`, `get_by_squad_number`, and `delete` now return `Result<T, PlayerServiceError>` (wrapping `diesel::result::Error` via `PlayerServiceError::Database`), not `diesel::result::Error` directly. Callers in `src/routes/players.rs` use wildcard error patterns (`map_err(|_| Status::InternalServerError)`, `Err(_) =>`), so they required no modification. Tests assert only with `is_ok()`, `is_err()`, `unwrap()`, and `is_none()` — no tests pattern-match on `PlayerServiceError` variants.

Learnt from: nanotaboada
Repo: nanotaboada/rust-samples-rocket-restful PR: 74
File: src/services/player_service.rs:3-10
Timestamp: 2026-04-08T01:46:25.312Z
Learning: In `src/services/player_service.rs`, the service layer intentionally uses `&mut diesel::SqliteConnection` in all public function signatures and returns `diesel::result::Error` directly for read operations (`get_all`, `get_by_id`, `get_by_squad_number`, `delete`). This is by design: services are the connection-passing boundary (routes acquire `&mut conn` from the r2d2 pool and pass it down), while repositories own all Diesel DSL and `crate::schema` references. Wrapping `diesel::result::Error` in a separate `ServiceError` enum for these functions was considered unnecessary indirection for this project.

Learnt from: nanotaboada
Repo: nanotaboada/rust-samples-rocket-restful PR: 49
File: tests/player_routes_tests.rs:70-79
Timestamp: 2026-03-29T17:09:18.097Z
Learning: In `tests/player_routes_tests.rs` (and the corresponding handler in src/routes/health.rs), the `health()` Rocket route handler returns only an HTTP `Status` with no response body. Calling `response.into_string()` on a bodyless Rocket response yields `None`, so `.unwrap()` would panic. Tests for this endpoint should assert only the status code (`Status::Ok`), not a response body.

Learnt from: CR
Repo: nanotaboada/rust-samples-rocket-restful PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-04-08T01:55:37.411Z
Learning: Applies to **/*.rs : Avoid `unwrap()`, `expect()`, unnecessary `.clone()`, blocking operations in async handlers, missing `?` error propagation, and inline comments between AAA test sections

Learnt from: CR
Repo: nanotaboada/rust-samples-rocket-restful PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-04-08T01:55:37.411Z
Learning: Applies to tests/**/*.rs : Use `player_request_for_creation()`, `player_request_for_update()`, and `EXISTING_PLAYER_ID` from `tests/common` for request bodies and UUID-based GET tests; never hardcode UUID strings inline

Learnt from: CR
Repo: nanotaboada/rust-samples-rocket-restful PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-04-08T01:55:37.411Z
Learning: Applies to src/routes/**/*.rs : Use UUID (v4) with serde features for surrogate key lookups via `GET /players/{uuid}`; use `squad_number` as natural key for all mutation routes: `PUT` and `DELETE /players/squadnumber/{squad_number}`

Learnt from: nanotaboada
Repo: nanotaboada/rust-samples-rocket-restful PR: 74
File: src/services/player_service.rs:3-10
Timestamp: 2026-04-08T01:46:25.312Z
Learning: In `src/services/player_service.rs`, the service layer intentionally uses `&mut diesel::SqliteConnection` in all public function signatures and returns `diesel::result::Error` directly for read operations (`get_all`, `get_by_id`, `get_by_squad_number`, `delete`). This is by design: services are the connection-passing boundary (routes acquire `&mut conn` from the r2d2 pool and pass it down), while repositories own all Diesel DSL and `crate::schema` references. Wrapping `diesel::result::Error` in a separate `ServiceError` enum for these functions was considered unnecessary indirection for this project.

Learnt from: CR
Repo: nanotaboada/rust-samples-rocket-restful PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-04-08T01:55:37.411Z
Learning: Applies to src/services/**/*.rs : Services must not reference Diesel or SQL directly; they must contain pure business logic and return `Result` types

Learnt from: nanotaboada
Repo: nanotaboada/rust-samples-rocket-restful PR: 74
File: src/services/player_service.rs:50-51
Timestamp: 2026-04-08T01:46:19.698Z
Learning: In `src/services/player_service.rs` (and related repository/route files), `squad_number` is a football squad number constrained to the range 1–99 (seed data uses 1–27). The `u32 as i32` cast is intentional and safe for this domain; flagging it as an overflow risk is a false positive. No validation guard for values above `i32::MAX` is needed.

Learnt from: nanotaboada
Repo: nanotaboada/rust-samples-rocket-restful PR: 74
File: src/services/player_service.rs:3-10
Timestamp: 2026-04-08T01:46:25.312Z
Learning: In `src/services/player_service.rs`, the module-level doc comment intentionally reads "connection-aware but DSL-free" to reflect that the service layer accepts `&mut SqliteConnection` but contains no Diesel query DSL (no `diesel::prelude`, no `crate::schema` imports). All persistence DSL lives in `src/repositories/player_repository.rs`.

Learnt from: nanotaboada
Repo: nanotaboada/rust-samples-rocket-restful PR: 55
File: src/state/player_collection.rs:34-40
Timestamp: 2026-04-05T00:23:45.165Z
Learning: In `src/state/player_collection.rs`, the `initialize_database()` function uses an emptiness check (`is_empty`) as a bootstrap signal rather than schema-based first-run detection. This is intentional for this PR: a committed pre-seeded DB is shipped, so the emptiness check only acts as a safety net for fresh clones. The deeper fix (schema-based first-run detection, transactional seed, `Result` propagation instead of `expect()`/`unwrap()`) is deferred to the upcoming migrations PR.